Skip to content

Fix device client type, update READMEs and e2e apps#543

Open
ancheetah wants to merge 2 commits intomainfrom
2.0-QA-pt2
Open

Fix device client type, update READMEs and e2e apps#543
ancheetah wants to merge 2 commits intomainfrom
2.0-QA-pt2

Conversation

@ancheetah
Copy link
Collaborator

@ancheetah ancheetah commented Mar 3, 2026

JIRA Ticket

https://pingidentity.atlassian.net/browse/SDKS-4816

Description

  • Fix device profile query type - realm should be optional.
  • Import client types into e2e apps to test type exports
  • Update READMEs with correct type imports

Summary by CodeRabbit

  • New Features

    • Device client now accepts realm as an optional parameter in device profile queries, providing greater configuration flexibility.
  • Documentation

    • Added comprehensive installation and quickstart guide for OIDC client package.
    • Improved import statement documentation for consistency across packages.

@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

This pull request adds type annotations across e2e test applications and package files, makes the realm property optional in the device profile types, updates import patterns in documentation, and introduces a changeset entry documenting the realm change for the device-client package.

Changes

Cohort / File(s) Summary
Changeset Entry
.changeset/soft-moose-read.md
Adds patch version bump for @forgerock/device-client documenting realm as optional in device profile query.
E2E Type Annotations
e2e/davinci-app/components/fido.ts, e2e/device-client-app/src/utils/index.ts, e2e/oidc-app/src/utils/oidc-app.ts, e2e/protect-app/src/protect-native.ts
Adds explicit type annotations for client instances (FidoClient, DeviceClient, OidcClient, Protect) imported from respective type modules; improves type safety without runtime changes.
Type Definitions
packages/device-client/src/lib/types/profile-device.types.ts
Changes realm property in GetProfileDevices interface from required to optional (realm?: string).
Documentation Updates
packages/device-client/README.md, packages/journey-client/README.md, packages/oidc-client/README.md
Reorganizes import patterns in Quick Start sections and adds Installation and Quickstart sections to oidc-client README with usage examples.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • feat: expose client return types #527: Introduces the exported client return types (DeviceClient, FidoClient, OidcClient, Protect) that are directly consumed by the type annotations added in this PR.

Suggested reviewers

  • ryanbas21
  • cerebrl

Poem

🐰 Hops through the code with glee,
Making realms optional, wild and free!
Types now guide each client's way,
Safer flows for e2e's day.
DeviceClient, OidcClient stand tall,
Well-typed companions through it all! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: fixing device client type (realm optional), updating READMEs, and modifying e2e apps with type imports.
Description check ✅ Passed The description covers all key points: JIRA ticket included, main objective stated (realm optional), and secondary objectives listed (type imports in e2e apps, README updates).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2.0-QA-pt2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@changeset-bot
Copy link

changeset-bot bot commented Mar 3, 2026

🦋 Changeset detected

Latest commit: 7bd1377

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@forgerock/device-client Patch
@forgerock/davinci-client Patch
@forgerock/journey-client Patch
@forgerock/oidc-client Patch
@forgerock/protect Patch
@forgerock/sdk-types Patch
@forgerock/sdk-utilities Patch
@forgerock/iframe-manager Patch
@forgerock/sdk-logger Patch
@forgerock/sdk-oidc Patch
@forgerock/sdk-request-middleware Patch
@forgerock/storage Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@nx-cloud
Copy link
Contributor

nx-cloud bot commented Mar 3, 2026

View your CI Pipeline Execution ↗ for commit 7bd1377

Command Status Duration Result
nx run-many -t build --no-agents ✅ Succeeded <1s View ↗
nx affected -t build lint test typecheck e2e-ci ✅ Succeeded 4m 18s View ↗

☁️ Nx Cloud last updated this comment at 2026-03-03 16:04:00 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 3, 2026

Open in StackBlitz

@forgerock/davinci-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/davinci-client@543

@forgerock/device-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/device-client@543

@forgerock/journey-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/journey-client@543

@forgerock/oidc-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/oidc-client@543

@forgerock/protect

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/protect@543

@forgerock/sdk-types

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-types@543

@forgerock/sdk-utilities

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-utilities@543

@forgerock/iframe-manager

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/iframe-manager@543

@forgerock/sdk-logger

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-logger@543

@forgerock/sdk-oidc

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-oidc@543

@forgerock/sdk-request-middleware

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-request-middleware@543

@forgerock/storage

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/storage@543

commit: 7bd1377

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 14.75%. Comparing base (b89ad58) to head (7bd1377).
⚠️ Report is 55 commits behind head on main.

❌ Your project status has failed because the head coverage (14.75%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #543      +/-   ##
==========================================
- Coverage   18.79%   14.75%   -4.05%     
==========================================
  Files         140      153      +13     
  Lines       27640    26265    -1375     
  Branches      980     1054      +74     
==========================================
- Hits         5195     3875    -1320     
+ Misses      22445    22390      -55     
Files with missing lines Coverage Δ
...evice-client/src/lib/types/profile-device.types.ts 100.00% <ø> (ø)

... and 49 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Deployed cd0a89f to https://ForgeRock.github.io/ping-javascript-sdk/pr-543/cd0a89fb9792555f580f0ee8046d5a7bbf75acc0 branch gh-pages in ForgeRock/ping-javascript-sdk

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🆕 New Packages

🆕 @forgerock/journey-client - 87.3 KB (new)
🆕 @forgerock/journey-client - 0.0 KB (new)

📊 Minor Changes

📈 @forgerock/device-client - 9.2 KB (+0.0 KB)

➖ No Changes

@forgerock/sdk-logger - 1.6 KB
@forgerock/sdk-request-middleware - 4.5 KB
@forgerock/iframe-manager - 2.4 KB
@forgerock/sdk-oidc - 4.8 KB
@forgerock/storage - 1.5 KB
@forgerock/sdk-types - 7.9 KB
@forgerock/protect - 150.1 KB
@forgerock/davinci-client - 41.3 KB
@forgerock/sdk-utilities - 11.2 KB
@forgerock/oidc-client - 24.9 KB


13 packages analyzed • Baseline from latest main build

Legend

🆕 New package
🔺 Size increased
🔻 Size decreased
➖ No change

ℹ️ How bundle sizes are calculated
  • Current Size: Total gzipped size of all files in the package's dist directory
  • Baseline: Comparison against the latest build from the main branch
  • Files included: All build outputs except source maps and TypeScript build cache
  • Exclusions: .map, .tsbuildinfo, and .d.ts.map files

🔄 Updated automatically on each push to this PR

@ancheetah ancheetah marked this pull request as ready for review March 3, 2026 22:18
@ancheetah ancheetah requested review from cerebrl and ryanbas21 March 3, 2026 22:18
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/oidc-client/README.md (1)

19-35: ⚠️ Potential issue | 🟡 Minor

Fix the token.exchange quickstart call signature.

The quickstart shows passing a single object, but the API expects positional code and state arguments as per the actual implementation.

📘 Suggested README fix
-const newTokens = await oidcClient.token.exchange({
-  /* code, state */
-}); // Returns new tokens or error
+const code = '...';
+const state = '...';
+const newTokens = await oidcClient.token.exchange(code, state); // Returns new tokens or error
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/oidc-client/README.md` around lines 19 - 35, The README example
incorrectly calls oidcClient.token.exchange with a single object; update the
quickstart to call the actual implementation signature by passing code and state
as positional arguments to oidcClient.token.exchange (e.g., call exchange(code,
state)), and update the surrounding comment to reflect it returns new tokens or
an error; locate the example block using the symbol oidcClient.token.exchange
and replace the single-object call with the positional-argument form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/oidc-client/README.md`:
- Around line 19-35: The README example incorrectly calls
oidcClient.token.exchange with a single object; update the quickstart to call
the actual implementation signature by passing code and state as positional
arguments to oidcClient.token.exchange (e.g., call exchange(code, state)), and
update the surrounding comment to reflect it returns new tokens or an error;
locate the example block using the symbol oidcClient.token.exchange and replace
the single-object call with the positional-argument form.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ea79b4 and 7bd1377.

📒 Files selected for processing (9)
  • .changeset/soft-moose-read.md
  • e2e/davinci-app/components/fido.ts
  • e2e/device-client-app/src/utils/index.ts
  • e2e/oidc-app/src/utils/oidc-app.ts
  • e2e/protect-app/src/protect-native.ts
  • packages/device-client/README.md
  • packages/device-client/src/lib/types/profile-device.types.ts
  • packages/journey-client/README.md
  • packages/oidc-client/README.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants